Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding new resource to manage group LDAP links #296

Merged
merged 2 commits into from
May 26, 2020

Conversation

jasoncarlsen
Copy link
Contributor

Closes #183

@ringods
Copy link
Contributor

ringods commented May 22, 2020

@jasoncarlsen if you could also document the new resource, I will merge your contribution. Please add it in the website/docs/r folder. You can take any peer file in that folder as an example on how to document resources.

@jasoncarlsen
Copy link
Contributor Author

@ringods Thanks for the feedback, I've added documentation as requested. Please let me know if I need to change anything else.

@ringods
Copy link
Contributor

ringods commented May 26, 2020

@jasoncarlsen at first sight this looks fine. Tnx for the work you did in this contribution.

@ringods ringods merged commit 3233172 into gitlabhq:master May 26, 2020
@ringods
Copy link
Contributor

ringods commented May 27, 2020

@jasoncarlsen after the merge, the acceptance tests of the Community Edition fail. One of the lines is regarding the LDAP link. Can you have a look and verify whether this is related or not?

https://github.com/terraform-providers/terraform-provider-gitlab/runs/710870254

@jasoncarlsen
Copy link
Contributor Author

jasoncarlsen commented May 27, 2020

@ringods I took a look and the test is behaving as expected. There are two things to keep in mind with this test:

  1. The test is being skipped (and thus throwing the warning) because the test data is set to the default values. I would need to know a real LDAP CN and the name of the configured LDAP provider in order to run the tests.
  2. GitLab CE doesn't support LDAP links so this test should only ever be run against EE.

@ringods
Copy link
Contributor

ringods commented May 27, 2020

@jasoncarlsen If the test should be skipped on CE, could you submit an additional PR where you use SkipFunc: isRunningInCE in the tests?

See here for an example:
https://github.com/terraform-providers/terraform-provider-gitlab/blob/da9cca35fe74e27c7fbdb248794e42e6c47344a0/gitlab/resource_gitlab_project_push_rules_test.go#L47

@jasoncarlsen
Copy link
Contributor Author

ahmet2mir pushed a commit to ahmet2mir/terraform-provider-gitlab that referenced this pull request Sep 15, 2020
Adding new resource to manage group LDAP links
@github-actions github-actions bot locked and limited conversation to collaborators Nov 10, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

Successfully merging this pull request may close these issues.

[Feature Request]: LDAP synchronisation functionality
2 participants